-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Rust implementation for Python client's update_run()
method.
#1314
Conversation
@@ -47,8 +47,7 @@ impl BlockingTracingClient { | |||
Ok(Self { client: Arc::from(client) }) | |||
} | |||
|
|||
// N.B.: We use `Py<Self>` so that we don't hold the GIL while running this method. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GIL info here was outdated, so I removed it. The current implementation drops the GIL explicitly by calling allow_threads()
.
} | ||
|
||
Ok(Some(attachments)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved this function down, to keep all the type definitions next to each other in the file. Otherwise it remained unchanged.
@@ -196,6 +213,45 @@ impl FromPyObject<'_> for RunCommon { | |||
} | |||
} | |||
|
|||
fn extract_attachments(value: &Bound<'_, PyAny>) -> PyResult<Option<Vec<Attachment>>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is unchanged, just moved farther down in the same file.
ref_name: key.to_string(), | ||
filename: key.to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's the ref_name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attachments dict is name -> (mime_type, data)
, so is using the key for both ref_name
and filename
the right thing to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a comment but otherwise lgtm
Implement the
update_run()
client method in the Rust PyO3 bindings, and call it from the Python client if the Rust bindings are enabled.